Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

The Great Line Ending & Cursor Range Cleanup #376

Merged

Conversation

cessen
Copy link
Contributor

@cessen cessen commented Jun 25, 2021

Addresses #362.

Switch all code over to:

  1. Use gap indexing.
  2. Not assume a file-final line-ending.

All code has been adjusted so that it still presents an on-char indexing model to the user, but internally everything works in terms of gap indexing.

@cessen cessen force-pushed the great_line_ending_and_cursor_range_cleanup branch 2 times, most recently from b194d54 to 9493688 Compare June 30, 2021 12:49
@cessen cessen force-pushed the great_line_ending_and_cursor_range_cleanup branch from 51d0168 to e725957 Compare July 1, 2021 21:24
@cessen cessen force-pushed the great_line_ending_and_cursor_range_cleanup branch from a1f9ec8 to b4c59b4 Compare July 8, 2021 23:47
@cessen cessen force-pushed the great_line_ending_and_cursor_range_cleanup branch from 774a8e1 to 954314a Compare July 17, 2021 18:03
@cessen cessen marked this pull request as ready for review July 19, 2021 06:11
@cessen cessen mentioned this pull request Jul 28, 2021
2 tasks
@cessen
Copy link
Contributor Author

cessen commented Jul 28, 2021

This is almost done. There's just one last bug I need to track down, where / searching doesn't seem to work correctly after CJK characters. (My guess is it's a mismatch between byte and char indices, but I haven't tracked it down yet.)

Forgot to convert from char indices to byte indices before passing
to the regex engine.
@cessen
Copy link
Contributor Author

cessen commented Jul 28, 2021

Yup, silly mistake on my part. Fixed!

I think this is ready for a final round of reviews, now.

@cessen cessen requested a review from archseer July 28, 2021 23:05
Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, after a bunch of testing I think this is good enough for merge, and we can deal with any residual issues on master. Great work! 🎉

@archseer archseer merged commit 05d20e1 into helix-editor:master Jul 29, 2021
@CBenoit
Copy link
Member

CBenoit commented Jul 29, 2021

Great work!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants